-
Notifications
You must be signed in to change notification settings - Fork 121
[Woo POS][Barcodes] i2 Use GameController framework for detecting scanner input #15877
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Woo POS][Barcodes] i2 Use GameController framework for detecting scanner input #15877
Conversation
Generated by 🚫 Danger |
|
|
Introduces `GameControllerBarcodeParser`, a new mechanism for processing input from physical barcode scanners that connect as game controllers. This approach offers a more reliable, language-independent method of barcode scanning by interpreting raw `GCKeyCode` values directly from the hardware. This bypasses the iOS keyboard layout system, which can interfere with HID-based scanning on international keyboards. The new parser's logic has been aligned with the existing `HIDBarcodeParser` to ensure identical behavior and maintain consistency across scanning methods. This includes mirroring the specific, nuanced timeout handling. A test suite, `GameControllerBarcodeParserTests`, has been added to validate all aspects of the parser's functionality and to lock in the behavioral parity with its HID counterpart.
Shift press and un-press events are reported by keyChangedHandler. We need to use those to determine whether the shift key needs to be applied to the next character.
…deScannerContainer
a1dd2e5 to
d8e1b48
Compare
| func makeUIViewController(context: Context) -> UIViewController { | ||
| let featureFlagService = ServiceLocator.featureFlagService | ||
|
|
||
| if featureFlagService.isFeatureFlagEnabled(.pointOfSaleBarcodeScanningi2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wondered if we should keep this for now or replace it entirely.
We could remove BarcodeScannerHostingController, HIDBarcodeParser, and possibly most if not all the BarcodeScannerContainer code.
GameControllerBarcodeObserver theoretically allows us to observe all the input within the aggregate model, although we would lose the ability to control in which screens we want to receive the input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I think we should keep the container. We already have the need to ignore scans in some screens (search, email receipt, everything in between building a cart and payment success.
The need for granular control is likely to increase over time – for example, any time we add a text field we need to think about it, and any control will be based on view state, what's focused. Quantity selectors, customer detail entry, configuring bookings or subscription purchases – there's lots of places we may end up having more complex text field management.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works really well. I've left a few small comments/questions in line, but all good.
The only thing I've noticed is a possibly increased tendency to miss the first character of a scan after the scanner's been asleep. It's hard to judge, but have you seen the same thing? If not, let's not worry about it – I don't have anything conclusive, and whenever I try to make it happen, it doesn't!
Oh, BTW it would be good to merge trunk first, and check the feature flag by hand, once you've decided about keeping the feature flag. I added one with the same name, further down, so we could get a mess.
| let featureFlagService = ServiceLocator.featureFlagService | ||
|
|
||
| if featureFlagService.isFeatureFlagEnabled(.pointOfSaleBarcodeScanningi2) { | ||
| return GameControllerBarcodeScannerHostingController( | ||
| configuration: configuration, | ||
| onScan: onScan | ||
| ) | ||
| } else { | ||
| return BarcodeScannerHostingController( | ||
| configuration: configuration, | ||
| onScan: onScan | ||
| ) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| let featureFlagService = ServiceLocator.featureFlagService | |
| if featureFlagService.isFeatureFlagEnabled(.pointOfSaleBarcodeScanningi2) { | |
| return GameControllerBarcodeScannerHostingController( | |
| configuration: configuration, | |
| onScan: onScan | |
| ) | |
| } else { | |
| return BarcodeScannerHostingController( | |
| configuration: configuration, | |
| onScan: onScan | |
| ) | |
| } | |
| return GameControllerBarcodeScannerHostingController( | |
| configuration: configuration, | |
| onScan: onScan | |
| ) |
I don't actually think this needs to be feature flagged, we can just ship it – we're changing the app to match Android i1 after all.
It is helpful for testing the PR though. If you want to keep the flag, perhaps rename it to something other than i2, so we can ship this before the rest of i2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it was helpful for testing. I didn't want to remove it right away. However, we could simply replace one with another without any feature flagging.
| // This prevents a double-trigger-pull on a Star scanner from adding an error row – | ||
| // Star use this as a shortcut to switch to the software keyboard. They send keycode 174 0xAE, which is | ||
| // undefined and reserved in UIKeyboardHIDUsage. The scanner doesn't send a character with the code. | ||
| // There seems to be no reason to handle empty input when considering scans. | ||
| return false | ||
| } | ||
|
|
||
| guard character.isNotEmpty else { | ||
| // This prevents a double-trigger-pull on a Star scanner from adding an error row – | ||
| // Star use this as a shortcut to switch to the software keyboard. They send keycode 174 0xAE, which is | ||
| // undefined and reserved in UIKeyboardHIDUsage. The scanner doesn't send a character with the code. | ||
| // There seems to be no reason to handle empty input when considering scans. | ||
| return false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this comment block to be repeated? Perhaps it could all be in a single guard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we don't. I'll clean it up.
| case .period: return isShiftPressed ? ">" : "." | ||
| case .slash: return isShiftPressed ? "?" : "/" | ||
| case .graveAccentAndTilde: return isShiftPressed ? "~" : "`" | ||
| case .returnOrEnter: return "\r" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should \n be in this list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it's returnOrEnter, I think they are both treated the same way with GameController framework.
@joshheald, interesting. I will test on my side a few times and see from the implementation point of view if there's any good reason for this to happen. |
|
@joshheald, thanks for testing again! I haven't been able to reproduce the first value missing issue myself. I rotated a dozen rounds of my scanner, going to sleep and disconnecting, and then scanning different types of barcodes. All of them succeeded. Let's keep observing it. I removed the feature flag and left GameController as the only way to get the scanned items while also keeping the container to only track scanning within certain views. |

Closes WOOMOB-755
Description
This PR implements GameController-based barcode scanning for POS to solve language-dependent input issues. The current HID approach causes barcode parsing problems when iPad keyboard language differs from the scanner's keyboard setting (e.g., barcode "1234" might be read as "&é"'" due to iOS keyboard configuration mismatch).
The business logic aims to be exactly the same as the current keyboard text input +
HIDBarcodeParsersetup.The implementation adds:
pointOfSaleBarcodeScanningi2HIDBarcodeParserlogic forGameControllerBarcodeParserTesting steps
Testing GameController Implementation (Feature Flag Enabled)
Testing information
Tested with iPad Air running iOS 26 using Inateck scanner and various 1D and 2D barcodes.
Testing session
Game.Controller.Scanning.MP4
RELEASE-NOTES.txtif necessary.